Skip to content

Unify security headers so CSP/HSTS aren't middleware-only (#368) - #548

Open
Vyacheslav-Tomashevskiy wants to merge 1 commit into
Northgate-Systems:mainfrom
Vyacheslav-Tomashevskiy:fix/368-security-headers-audit
Open

Unify security headers so CSP/HSTS aren't middleware-only (#368)#548
Vyacheslav-Tomashevskiy wants to merge 1 commit into
Northgate-Systems:mainfrom
Vyacheslav-Tomashevskiy:fix/368-security-headers-audit

Conversation

@Vyacheslav-Tomashevskiy

Copy link
Copy Markdown
Contributor

Closes #368.

What was actually wrong

security-edge.ts's applySecurityHeaders() — the one wired into middleware.ts — was missing Content-Security-Policy and Strict-Transport-Security entirely. Those two only existed in a second, hand-copied header list in next.config.ts.

I checked live (ran next dev, curled /, /login, /dashboard, /api/health, a 401, a 500, a 404, and /favicon.ico) whether anything was actually missing from a real response. next.config.ts's headers() config covers every path (source: "/(.*)"), so in practice all 5 headers named in the issue were present everywhere — but the two lists had already drifted apart:

  • next.config.ts: X-Frame-Options: SAMEORIGIN, Cross-Origin-Opener-Policy: same-origin-allow-popups
  • security-edge.ts: X-Frame-Options: DENY, Cross-Origin-Opener-Policy: same-origin

So the site was actually running two different header policies depending on whether the request path was covered by the middleware matcher (config.matcher in middleware.ts excludes _next/static, images, .css/.js, etc.) or fell through to next.config.ts alone — a static asset got the weaker values, everything else got the stricter ones set later by middleware. That divergence is exactly the kind of gap "not just some routes" is pointing at, and it would only get worse as either file gets edited independently in the future.

Fix

  • security-edge.ts now exports SECURITY_HEADERS, a single array with all 7 headers (including the previously-missing CSP and HSTS), and applySecurityHeaders() just iterates it.
  • next.config.ts imports SECURITY_HEADERS instead of keeping its own copy, so excluded/static paths now get the exact same (stricter) values as everything else.
  • Removed the dead, unused, already-diverged duplicate of applySecurityHeaders / isAllowedOrigin / ALLOWED_ORIGINS in security.ts — confirmed via grep it's never imported anywhere, and it disagreed with security-edge.ts on the CORS allowlist (missing the Vercel preview domain, opposite null-origin handling). Leaving it in place is exactly what let the header values drift apart in the first place.
  • Added src/lib/__tests__/security-edge.test.ts: all 5 required headers present, no duplicate keys, correct CSP default-src, strict X-Frame-Options, applySecurityHeaders applies every entry (not a hand-picked subset), and isAllowedOrigin/ALLOWED_ORIGINS stay in sync.

Verification

  • npx tsc --noEmit — no new errors (pre-existing unrelated errors only: the tracked isValidStellarPublicKey missing-import bug in fix(validations): restore broken isValidStellarPublicKey import; feat: add /api/stellar/fee-estimate #529, and a pre-existing type mismatch in a transactions test).
  • Temporarily restored the missing import from fix(validations): restore broken isValidStellarPublicKey import; feat: add /api/stellar/fee-estimate #529 locally to run the full suite end to end, then reverted it before this commit (git diff on validations.ts is empty in this PR):
    • Baseline (git stash, unmodified main): 4 failed / 2 passed test files, 10 tests.
    • With this PR's changes: 6 passed / 1 failed test files, 77 tests (75 passing). The 1 remaining failure is a pre-existing bad fixture address in validations.test.ts (stellarSendSchema), unrelated to this change.
  • npx eslint on all touched files — clean.
  • Live-curled next dev before and after: confirmed X-Frame-Options and Cross-Origin-Opener-Policy now match exactly between a middleware-covered route (/) and a middleware-excluded static path (/favicon.ico), both now also carrying full CSP + HSTS.

/claim

…nly (Northgate-Systems#368)

security-edge.ts's applySecurityHeaders() (the one wired into middleware.ts)
was missing Content-Security-Policy and Strict-Transport-Security entirely -
those two only existed as a second, hand-copied header list in
next.config.ts. Confirmed live (next dev + curl) that next.config.ts's
headers() does cover every response including 401/403/500/404s, so nothing
was actually missing in practice, but the two lists had already drifted:
next.config.ts had X-Frame-Options: SAMEORIGIN and
Cross-Origin-Opener-Policy: same-origin-allow-popups while security-edge.ts
used the stricter DENY / same-origin - two different policies for the same
site depending on whether the request path was covered by the middleware
matcher (config.matcher in middleware.ts excludes _next/static, images,
.css/.js, etc.) or fell through to next.config.ts alone.

- security-edge.ts now exports SECURITY_HEADERS, a single array with all 7
  headers (including the CSP and HSTS that were missing) and is the one
  place applySecurityHeaders() and next.config.ts both read from.
- next.config.ts imports SECURITY_HEADERS instead of keeping its own copy,
  so a static asset that skips the middleware now gets the exact same
  (stricter) values instead of the old weaker ones.
- Removed the dead, unused, already-diverged duplicate of
  applySecurityHeaders/isAllowedOrigin/ALLOWED_ORIGINS in security.ts (never
  imported anywhere, confirmed via grep) - it disagreed with security-edge.ts
  on the CORS allowlist (missing the Vercel preview domain, null-origin
  handling flipped) and was exactly the kind of copy that caused this bug in
  the first place.
- Added src/lib/__tests__/security-edge.test.ts covering the 5 headers named
  in the issue, header-value consistency, and isAllowedOrigin.

Verified against the documented isValidStellarPublicKey / PR Northgate-Systems#529 baseline:
temporarily restored the missing import in validations.ts to run the suite
(4 failed/2 passed files, 10 tests baseline -> 6 passed/1 failed files, 77
tests with these changes; the 2 failures are a pre-existing bad test fixture
address unrelated to this change), then reverted the import before this
commit (git diff on validations.ts is empty).
@vercel

vercel Bot commented Sep 10, 2026

Copy link
Copy Markdown

Someone is attempting to deploy a commit to the codex723's projects Team on Vercel.

A member of the Team first needs to authorize it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add security headers via src/lib/security-edge.ts audit

1 participant